Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve pyrex cython linter. #1675

Merged
merged 3 commits into from Jun 27, 2018
Merged

Improve pyrex cython linter. #1675

merged 3 commits into from Jun 27, 2018

Conversation

nicopauss
Copy link
Contributor

Like many other linters, use variables for the executable and options
used by the linter.
By default, the linter now report every warnings as errors with
--warning-errors.
Also add include directory and set working directory to file directory.

Like many other linters, use variables for the executable and options
used by the linter.
By default, the linter now report every warnings as errors with
`--warning-errors`.
Also add include directory and set working directory to file directory.
" Description: cython syntax checking for cython files.

call ale#Set('pyrex_cython_executable', 'cython')
call ale#Set('pyrex_cython_options', '--warning-extra --warning-errors')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove --warning-errors here, so we can preserve the current behavior? If users want warnings to be errors, they can change the option themselves. I don't want to change the behavior without a very compelling reason, and whether warnings should be errors is subjective.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Add a custom handler to support cython warning format.
Remove '--warning-errors' to keep previous behaviour.
Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, that'll be more agreeable for people. Now add a test for the GetCommand function. Have a look at tests in test/command_callback, maybe at the tests for Java and a few other things.

Add common callback tests to check if executable and options are well
configurable.
@nicopauss
Copy link
Contributor Author

Done

@w0rp
Copy link
Member

w0rp commented Jun 27, 2018

Good stuff. 👍 Cheers! 🍻

@w0rp w0rp merged commit 980aa35 into dense-analysis:master Jun 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants